Skip to content

Improve addons integration#2367

Closed
jubnzv wants to merge 18 commits into
cppcheck-opensource:masterfrom
jubnzv:improve-addons-integration
Closed

Improve addons integration#2367
jubnzv wants to merge 18 commits into
cppcheck-opensource:masterfrom
jubnzv:improve-addons-integration

Conversation

@jubnzv
Copy link
Copy Markdown
Contributor

@jubnzv jubnzv commented Nov 16, 2019

This PR improves current implementation of addons in cppcheck lib. Now we have separate class for addons configuration. That allows us write tests for addons functionality and makes it easier to add new features and handle errors.

For example, we can check environment (e.g. Python version/dependencies), handle errors related to json parsing (see #2362 (comment)) in one place.

My idea is use addonsutils.cpp to provide more utilities for addons integration and use it in both CLI and GUI. I'm currently working on GUI integration, so for now, leave this PR as draft.

What do you think @danmar? Does these changes are acceptable?

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Nov 16, 2019

I did not look very closely but I like the idea. 👍

@jubnzv
Copy link
Copy Markdown
Contributor Author

jubnzv commented Nov 16, 2019

I did not look very closely but I like the idea. +1

Ok. It takes some time to finish it.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Nov 16, 2019

That is ok. It seems best to merge it after the release.

@amai2012 amai2012 added the merge-after-next-release Wait with merging this PR until after the next Release label Nov 16, 2019
@jubnzv jubnzv force-pushed the improve-addons-integration branch from 5de0b6b to eeb1e5d Compare November 17, 2019 15:00
Comment thread lib/addonutils.cpp
throwAddonError("Failed to open " + fileName);

picojson::value json;
fin >> json;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not forget to merge the fix adding json-parser error checking here ;)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll take a look at this. Really I'm waiting for next release, to merge master changes in my branch. And somehow fix windows build.

@jubnzv jubnzv closed this Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-after-next-release Wait with merging this PR until after the next Release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants